Skip to content

Added CI workflow / Fixed IsDirCheck and ExistCheck methods #10

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
May 19, 2021

Conversation

cmaglie
Copy link
Member

@cmaglie cmaglie commented May 18, 2021

Added the CI run for all OS, and it already discovered two bugs.

1. ExistCheck() sometimes returns an error when it should not

Looking at how ExistCheck is defined:

// ExistCheck return true if the path exists or false if the path doesn't exists.
// In case the check fails false is returned together with the corresponding error.

It happens that on UNIX if a file already exist, say path/to/file, and you try to Stat using file as a directory:

info, err := os.Stat("path/to/file/name")

then Stat returns an error that is NOT an os.ErrNotExist. In this particular case the error is a os.PathError with Err field set to ENOTDIR. We now explicitly check for this particular case and the test case has been updated accordingly.

After updating the test case, it passes also on Windows.

2. IsDirCheck() sometimes do return errors when it should

Looking at how IsDirCheck is defined:

// IsDirCheck returns true if the path exists and is a directory or false
// if the path exists and is not a directory. In all the other case false and
// the corresponding error is returned.

in other words:

  • if the path exists and is a directory => true, nil
  • if the path exists and is NOT a directory => false, nil
  • if the path DOESN'T exists => false, err

This wasn't happening on UNIX (we had the same problem as 1. but with the opposite outcome).
Again after fixing this bug and updating the unit test it pass also on Windows.

@codecov-commenter
Copy link

codecov-commenter commented May 18, 2021

Codecov Report

❗ No coverage uploaded for pull request base (master@0430fee). Click here to learn what that means.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##             master      #10   +/-   ##
=========================================
  Coverage          ?   65.54%           
=========================================
  Files             ?        4           
  Lines             ?      267           
  Branches          ?        0           
=========================================
  Hits              ?      175           
  Misses            ?       64           
  Partials          ?       28           
Flag Coverage Δ
unit 65.54% <0.00%> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.


Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0430fee...4b51e48. Read the comment docs.

@cmaglie cmaglie changed the title Added CI workflow Added CI workflow / Fixed some IsDirCheck ExistCheck methods May 18, 2021
@cmaglie cmaglie changed the title Added CI workflow / Fixed some IsDirCheck ExistCheck methods Added CI workflow / Fixed IsDirCheck and ExistCheck methods May 18, 2021
@cmaglie cmaglie requested a review from silvanocerza May 18, 2021 14:30
@cmaglie cmaglie merged commit 5a57733 into master May 19, 2021
@cmaglie cmaglie deleted the add_ci_runner branch May 19, 2021 14:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants